Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Array<int> to options, additional limiters for grids (rebase) #2767

Open
wants to merge 6 commits into
base: next
Choose a base branch
from

Conversation

bendudson
Copy link
Contributor

Rebase of #2721 on next

  • Adds Array to the Options type
  • Reads Array from NetCDF files (e.g. mesh input file), and makes them available through Mesh::get

The reason this was added is to be able to include additional limiters in the input file. The format is a short-term solution only, though these have a habit of becoming permanent.

bendudson and others added 4 commits October 12, 2023 08:26
Enable 1D arrays of integers to be read from NetCDF files,
and made available via `Mesh::get()`. Mainly useful for
mesh construction.
When reading 1D vector of ints, check that the input is long enough,
and resize the output vector.
A short-term fix, allowing extra limiters to be added with inputs:

limiter_count : int
limiter_yinds : [int], length limiter_count
limiter_xstarts : [int], length limiter_count
limiter_xends : [int], length limiter_count

The limiter(s) are added between yinds[i] and yinds[i] + 1
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

// Ensure that output variable has the correct size
var.resize(len);

const auto* it = std::begin(full_var);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable name 'it' is too short, expected at least 3 characters [readability-identifier-length]

  const auto* it = std::begin(full_var);
              ^

int limiter_count = 0;
Mesh::get(limiter_count, "limiter_count", 0);
if (limiter_count > 0) {
std::vector<int> limiter_yinds, limiter_xstarts, limiter_xends;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]

Suggested change
std::vector<int> limiter_yinds, limiter_xstarts, limiter_xends;
std::vector<int> limiter_yinds;
std::vector<int> limiter_xstarts;
std::vector<int> limiter_xends;

}

for (int i = 0; i < limiter_count; ++i) {
int yind = limiter_yinds[i];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'yind' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change
int yind = limiter_yinds[i];
int const yind = limiter_yinds[i];


for (int i = 0; i < limiter_count; ++i) {
int yind = limiter_yinds[i];
int xstart = limiter_xstarts[i];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'xstart' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change
int xstart = limiter_xstarts[i];
int const xstart = limiter_xstarts[i];

for (int i = 0; i < limiter_count; ++i) {
int yind = limiter_yinds[i];
int xstart = limiter_xstarts[i];
int xend = limiter_xends[i];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'xend' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change
int xend = limiter_xends[i];
int const xend = limiter_xends[i];

Copy link
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test would be nice 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants